fix(parquet): keep DELTA_BYTE_ARRAY dedup for values larger than the page size limit - #10505
fix(parquet): keep DELTA_BYTE_ARRAY dedup for values larger than the page size limit#10505adriangb wants to merge 1 commit into
Conversation
…page limit A `BYTE_ARRAY` value larger than `data_page_size_limit` exceeds the limit on its own, so the post-write `should_add_data_page` check cuts a page after every single value. Parquet requires at least one value per data page, so that value is unsplittable and the limit is simply unsatisfiable there. For `DELTA_BYTE_ARRAY` this is destructive rather than merely wasteful: each page boundary discards the encoder's previous value, so every value gets `prefix_length = 0` and the encoding degenerates to exactly `PLAIN`. Columns of large values that share long prefixes stop being deduplicated (apache#10489). Exempt a page's mandatory first value from the byte limit when that value alone already exceeds it, so the limit applies to what follows — the bytes we can still place on another page. The exemption is gated on `ColumnValueEncoder::compresses_against_previous_value`, so only `DELTA_BYTE_ARRAY` opts in; `PLAIN` and `DELTA_LENGTH_BYTE_ARRAY` cost the same wherever a value lands and keep their existing one-value page bound. Pages therefore stay bounded by the value size rather than growing with `write_batch_size`, preserving the fix from apache#9972. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
run benchmark arrow_writer |
|
run benchmark writer_overhead |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (d053bf0) to cd17899 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (d053bf0) to cd17899 (merge-base) diff Run configurationrun benchmark writer_overheadBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_overhead File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (d053bf0) to cd17899 (merge-base) diff Run configurationrun benchmark writer_overheadCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (d053bf0) to cd17899 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
DELTA_BYTE_ARRAYstops deduplicating in parquet 59 for values larger than the page size limit #10489The cause is the page flush, not the mini-batch splitting
The issue attributes the regression to #9972's byte-budget sub-batching. It's actually the page flush; the sub-batching only exposes it.
should_add_data_pagefires whenestimated_data_page_size() >= data_page_size_limit. ForDELTA_BYTE_ARRAYthat estimate is the real encoded size, and the first value on a page is stored in full — so a single 8 MiB value against a 1 MiB limit puts the page over the limit by itself and triggers a flush. Flushing clears the encoder'sprevious, so the next value getsprefix_length = 0, and so on. The encoding degenerates to exactlyPLAIN.Before #9972 a 1024-row mini-batch meant the check simply didn't run until 1024 values had been written, so the dedup survived. That was an accident of
write_batch_size, not a designed property — the same column at 2000 rows already lost the prefix at the 1024-value boundary.This matters for choosing a fix: making the byte budget encoding-aware (suggestion 2 in the issue) does not fix the reported bug. However precise the budget, the first value alone still exceeds the limit and still triggers the flush.
The fix
Parquet requires at least one value per data page, so a value larger than the limit cannot be split out — the limit is unsatisfiable for it. Counting those bytes against the limit is what forces the pathological one-value-per-page cut.
Record that first value's encoded size in
PageMetrics::page_size_floorand apply the limit to what follows it, i.e. the bytes that can still go on another page.The exemption is gated on a new
ColumnValueEncoder::compresses_against_previous_value(defaultfalse,trueonly forDELTA_BYTE_ARRAYin both the generic and arrow encoders).PLAINandDELTA_LENGTH_BYTE_ARRAYcost the same wherever a value lands, so there is nothing to preserve by keeping values together and they keep their existing tighter one-value page bound. All three regression tests from #9972 pass unmodified.Why not "don't split when the split cannot help"
Suggestion 1 in the issue restores the dedup, but by removing the bound that #9972 added, for exactly the workload it was added for. Measured with
Some(1) => chunk_sizeinbyte_budget_chunker.rs, 2048 rows × 128 KiB distinct values against a 64 KiB page limit — the same regime as a 10 MB value against the 1 MiB default, scaled to fit in RAM:Page size under suggestion 1 is
write_batch_size × value_size. At 1024 × 10 MB that is a 10 GB page, which is the failure #9972 fixed.Results
The reporter's table, reproduced verbatim (10 identical 8 MiB values, raw input 80 MiB):
data_page_size_limitValues that merely share a long prefix behave the same. Values sharing no prefix are unaffected in file size and stay bounded at up to two values per page — the exempt one plus the one that trips the budget.
Tests
test_column_writer_delta_byte_array_dedups_large_shared_prefix_values— 16 identical 64 KiB values, 16 KiB page limit. Fails onmainwith 1 MiB across 16 pages (byte for byte whatPLAINproduces); passes here at ~one value's worth.test_column_writer_delta_byte_array_bounds_pages_without_shared_prefix— the same column with values differing from byte 0, asserting pages stay bounded by two values. Guards against fixing this by dropping the bound.test_large_string_delta_byte_array_shared_prefix— theArrowWriterpath from the report, as an exact page-layout assertion.Notes
A follow-up worth considering separately: the byte budget in
count_within_budget_*still measures raw payload length, soDELTA_BYTE_ARRAYcolumns sub-batch more eagerly than the encoded size warrants. That is a throughput question rather than a correctness one, and it is not what caused this regression.🤖 Generated with Claude Code